Skip to content

Decouple host drivers and shutdown - #2702

Open
Arc676 wants to merge 1 commit into
NVIDIA:mainfrom
Arc676:decouple-driver-host-shutdown
Open

Decouple host drivers and shutdown#2702
Arc676 wants to merge 1 commit into
NVIDIA:mainfrom
Arc676:decouple-driver-host-shutdown

Conversation

@Arc676

@Arc676 Arc676 commented Jul 31, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown

Description

Decouple the host shutdown flag from the host driver flag. This adds support for systems that have the NVIDIA drivers installed locally but do not run systemd, for example Talos. The user can install the drivers via system extension but set WITH_SHUTDOWN_HOST_GPU_CLIENTS=false via Helm values to disable all code paths in the MIG manager that depend on systemd.

This requires a change to the chart because the entrypoint for the MIG manager is managed by the operator and the variable is set here. The user can't override the value or modify the ConfigMap; the former approach would have no effect and the latter approach would be immediately reverted by the operator.

See discussions in NVIDIA/mig-parted#382 and the parent issue NVIDIA/mig-parted#356.

Checklist

  • No secrets, sensitive information, or unrelated changes

  • Lint checks passing (make lint)

    • Skipped since no Go changes

  • Generated assets in-sync (make validate-generated-assets)

    • Didn't work, although not sure why 
       - Generating CRDs from the codebase
 $PATH_TO_REPO/bin/controller-gen rbac:roleName=gpu-operator-role crd webhook paths="./..." output:crd:artifacts:config=config/crd/bases
 -: build constraints exclude all Go files in $PATH_TO_REPO/vendor/github.com/cyphar/filepath-securejoin/pathrs-lite
 Error: not all generators ran successfully
 run `controller-gen rbac:roleName=gpu-operator-role crd webhook paths=./... output:crd:artifacts:config=config/crd/bases -w` to see all available markers, or `controller-gen rbac:roleName=gpu-operator-role crd webhook paths=./... output:crd:artifacts:config=config/crd/bases -h` for usage
 make: *** [manifests] Error 1

  • Go mod artifacts in-sync (make validate-modules)

    • Again, no code changes

  • Test cases are added for new code paths

    • No new code paths

Testing

Tested manually on a Kubernetes cluster running the aforementioned patch.

Summary by CodeRabbit

  • Bug Fixes
    • Preserved explicitly configured GPU client shutdown behavior while providing a fallback based on the host driver status when no value is supplied.

@copy-pr-bot

copy-pr-bot Bot commented Jul 31, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@Arc676
Decouple host drivers and shutdown
5aa78c1 
Don't assume that systemd clients must be shut down if the host has
drivers

Signed-off-by: Alessandro Vinciguerra <alessandro.vinciguerra@postfinance.ch>
@Arc676
Arc676 force-pushed the decouple-driver-host-shutdown branch from ac78991 to 5aa78c1 Compare July 31, 2026 07:34
@myeolenv

myeolenv commented Aug 1, 2026

Copy link
Copy Markdown

Thanks for the contribution. We will review this PR.

@rajathagasthya

rajathagasthya commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

/ok-to-test 5aa78c1

@rajathagasthya

rajathagasthya commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Thanks for the follow-up PR @Arc676! As mentioned in NVIDIA/mig-parted#382 (comment), we'll take a look at this after the code freeze period is over.

@rajathagasthya

rajathagasthya commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 4, 2026 •
edited
Loading

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented Sep 4, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 6e8155dd-8cc8-43b9-8330-58f5a26b9849

📥 Commits

Reviewing files that changed from the base of the PR and between 7a03b57 and 5aa78c1.

📒 Files selected for processing (1)
  • assets/state-mig-manager/0420_configmap.yaml

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.

Walkthrough

The ConfigMap entrypoint now preserves an externally set WITH_SHUTDOWN_HOST_GPU_CLIENTS value. It uses IS_HOST_DRIVER only when the variable is unset or empty.

Changes

GPU client shutdown configuration

Layer / File(s) Summary
Preserve shutdown setting
assets/state-mig-manager/0420_configmap.yaml		 The export uses ${WITH_SHUTDOWN_HOST_GPU_CLIENTS:-$IS_HOST_DRIVER}. An existing value takes precedence over IS_HOST_DRIVER.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Merge Risk: ⚪ Minimal · up to 5aa78

The entrypoint now preserves an explicitly configured GPU-client shutdown setting while retaining the existing host-driver fallback. No current merge-blocking risk is identified.

Poem

A rabbit checked the setting bright
The chosen value stayed in sight
A fallback waits when none is found
The host driver keeps the ground
GPU clients follow the right command
And hop safely across the land

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: decoupling host driver state from host shutdown behavior.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rajathagasthya rajathagasthya rajathagasthya approved these changes
@tariq1890 tariq1890 tariq1890 approved these changes
@cdesiniotis cdesiniotis Awaiting requested review from cdesiniotis cdesiniotis is a code owner
@karthikvetrivel karthikvetrivel Awaiting requested review from karthikvetrivel karthikvetrivel is a code owner
@rahulait rahulait Awaiting requested review from rahulait rahulait is a code owner
@shivamerla shivamerla Awaiting requested review from shivamerla shivamerla is a code owner

Assignees

@rajathagasthya rajathagasthya

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Arc676 @myeolenv @rajathagasthya @tariq1890